provide stable machine interface for ini on command line with json #19655
Conversation
| fwrite(ZSTR_VAL(buf.s), 1, ZSTR_LEN(buf.s), stdout); | ||
| fwrite("\r\n", 1, sizeof("\r\n"), stdout); | ||
| } else { | ||
| fprintf(stderr, |
There was a problem hiding this comment.
I'm not certain this can actually happen.
If anyone can show convincingly that this is pointless we should drop it ...
Maybe it's better to use throw on error, but I prefer to print something specifically to stderr because of how this interface will be used, still I can't imagine how an error could occur in the first place ... but this is probably a failure of my imagination rather than a fact.
|
This is bit tricky to write tests for because it relies on build configuration, writing a test that passes everywhere would not be a very valuable test ... everything used is extremely stable API ... If a test is proposed, I'll add it. |
krakjoe
force-pushed
the
krakjoe/ini-json
branch
from
9398463 to
8247c6d
Compare
If you would like this in PHP 8.5, I'm not comfortable with approving this without any kind of testing. You can try adding a test to run-extra-tests.php that confirms that the output |
|
It's possible we could add a test that executes the thing, but doesn't actually depend on the output, this makes sure that it doesn't fault ? Beyond that we are just testing that json works anyway ? |
|
You can't test part of the JSON output? Making sure it includes some specific values? |
krakjoe
force-pushed
the
krakjoe/ini-json
branch
from
8247c6d to
7a8e832
Compare
After a bit of digging, yes I can ... test added ... good job you pushed, because I had made a mistake ;) |
krakjoe
force-pushed
the
krakjoe/ini-json
branch
3 times, most recently
from
7e0fb97 to
ce5e43c
Compare
|
@php/release-managers-85 test passing everywhere, ready for review ... |
krakjoe
force-pushed
the
krakjoe/ini-json
branch
from
ce5e43c to
2a097a2
Compare
alternative #18583